tests/lvs: let the reference netlist decide which labels are pins, and the caller pick the gf180 MIM option - #112
Conversation
Cell LVS on main fails on diff_pair, diff_pair_ibias and opamp, all with the same shape: "extra top-level pin(s) in layout" for SUB and VTAIL. A pin label is not a property of a cell, it is a property of how the cell is used. VTAIL is a top-level pin of a standalone diff_pair and an internal net inside diff_pair_ibias. Elementary cells emit labels so they can be LVS'd on their own, and a parent that flattens them inherits those names. Fixing that in the generator means every composite has to suppress its children's labels at every level of the hierarchy. `with_pin_labels` does this for diff_pair, but the parameter stops there: diff_pair_ibias does not accept it and emits labels of its own, so opamp cannot suppress anything. Each new composite is one more place that has to remember, and one that forgets fails silently -- which is how these three got here. The reference netlist already states which names are pins. Honour that when staging inputs: drop labels whose text is not a port of the top `.subckt`. No cell has to cooperate, it works at any depth and whether or not the parent flattened, and it needs no ambient state -- relevant because `@cell` keys its cache on arguments, so a context manager or env var can hand back a stale component built under the opposite setting. Only the staged copy is filtered; the GDS a cell ships keeps its labels, so LEF and macro flows are unaffected. Matching is case-insensitive: generators and schematics disagree on capitalisation in practice (`vdd` vs `Vdd`). Dropping every label is reported as a warning rather than passed over: that is not inheritance but a naming mismatch, and silently leaving the layout with no pins would turn it into a confusing LVS failure downstream. With no readable ports, nothing is dropped. Measured on nine real cells, 5-12 labels over 49-4913 polygons: 0.1-1.0 ms to filter, ~27 ms including the GDS round trip, against 1-8 s per cell of LVS.
Pure Python -- no PDK, no klayout, no GDS toolchain -- so it runs wherever pytest does. That matters here: the LVS workflow is triggered by `workflow_run` off Cell DRC, which only fires on the default branch, so LVS never runs on a pull request. A PR that fixes or breaks LVS cannot be seen either way until after it merges, which is how main went red. Six cases, including the two that are easy to get wrong: matching has to ignore case, because generators and schematics disagree in practice (`vdd` vs `Vdd`) and a case-sensitive compare would strip real pins; and an unreadable port list must drop nothing rather than leave the layout with no pins at all. Both are checked by mutation: making the compare case-sensitive, or removing the empty-ports guard, each fails exactly one test.
|
Added a unit test for the filter — six cases, pure Python, no PDK or klayout needed. Two of them are the ones that are easy to get wrong. Matching has to ignore case, because generators and schematics disagree in practice ( Worth flagging separately: the LVS check cannot run on this PR, or on any PR. That is also why |
The repo has 14 tests under tests/ and no workflow executes any of them. That matters more than it looks: the DRC and LVS workflows run the gdsfactory backend (CPython 3.10, gdsfactory 7.7), so gdstk has no CI coverage at all today, and conftest.py pins the test suite to gdstk. This job is the first thing that exercises it. It also gives pull requests a check they can currently fail. lvs.yml is triggered by `workflow_run` off Cell DRC, which only fires on the default branch: LVS has runs on main and none on any fork branch, so a PR that fixes or breaks LVS cannot be seen either way until after it merges. Deliberately not `pip install -e .`: install_requires pins gdsfactory<=7.7.0 and numpy<=1.24.0, which forces CPython 3.10 and pulls the whole layout stack. None of it is needed -- verified in a clean venv with only pytest, gdstk, numpy, pandas, pydantic and docopt: 19 passed in 7.2 s. tests/test_cells_layout.py is left out. It builds every cell in both backends, takes ~85 s, and carries five documented xfails, so green there would mean "nothing changed" rather than "everything works". Cell-build coverage deserves its own argument, and its own PR.
setup-python's `cache: pip` looks for requirements.txt or pyproject.toml to hash and fails the job when it finds neither. This repo ships setup.py.
Pin markers were drawn on hardcoded sky130 layer numbers -- met1_pin (67,16), met1_label (67,5), met2_pin (68,16), met2_label (68,5) -- which mean nothing on gf180. They now come from the PDK. They were also sized and placed so that the marker carried only the label layer, tangent to the port's edge rather than centred on it. The extractor then finds no conductor under the text, the net comes out unnamed, and LVS reports the pin as missing from the layout. The marker is now sized to the routable metal's minimum width and aligned centre-to-centre, so it lands inside metal the router already placed. Rebased onto current main. The `no_pin_labels()` context manager this branch originally added is dropped: ReaLLMASIC#105 landed `with_pin_labels` on diff_pair, and ReaLLMASIC#112 handles the general case in the LVS runner, where it needs no cooperation from any cell and does not depend on ambient state that `@cell` cannot see.
Listing the four files reproduces the problem this job exists to fix: a test added later is not run until someone remembers to edit the workflow. ReaLLMASIC#100 adds tests/test_narrow_fets.py, which the explicit list would have silently skipped. Everything under tests/ now runs except test_cells_layout.py, which stays out for the reason already documented above the step.
The runner hardcoded option A, so a cell drawing its MIM on option B (met4/FuseTop/met5) extracted no capacitor at all and every MIM reported as missing from the layout. The two options are mutually exclusive at process level, so the deck can only be told one of them. Adds --mim-option / $GF180_MIM_OPTION, defaulting to A as before, and applies the option-A deck repair only when option A is selected -- the deck's option B branch already connects all three plates.
|
Added one more commit to this branch, since it lands in the same file and the same harness.
The commit adds Found while getting a gf180 cell drawn on option B through this harness: it went from The 6 unit tests on this branch still pass. Say the word if you would rather have this as a separate PR and I will split it back out. |
|
Great, thanks! |
Two notebooks sit on the 180s per-cell limit and cross it depending on
which runner the job lands on, so the check passes or fails on identical
content. `lvs-pin-filter` has four green runs and two red ones.
GLayout_Cells 290s pass (#112) / 372s error (#113)
glayout_opamp 136s pass (#112) / 198s error (#113)
The native backend builds these cells several times faster, which moves
them off the limit instead of raising it. Measured on this fork's CI --
same container, same runners -- with #102, #104 and #113 applied on both
sides, the only difference being this line:
gdsfactory gdstk
GLayout_Cells 187.8s ERROR 14.2s pass
glayout_opamp 126.4s pass 19.2s pass
5T_OTA_part2 90.8s pass 16.2s pass
test_bjt_custom_pattern 68.2s pass 11.4s pass
whole suite 660s 13/14 158s 14/14
Note the baseline already carries all three PRs and still fails: they are
what lets the notebooks run on gdstk at all, not what fixes the timeouts.
Depends on #102, #104 and #113. Without #113 `GLayout_Cells` raises in
`mimcap.py`; without #102 it raises in `Component.add`; without #104's
`add_ref(columns=)` the BJT notebook raises. This check stays red until
all three land.
Cell LVS on
mainfails on three cells, all with the same shape:A pin label is not a property of a cell, it is a property of how the cell is
used.
VTAILis a top-level pin of a standalonediff_pairand an internalnet inside
diff_pair_ibias. Elementary cells emit labels so they can be LVS'don their own, and a parent that flattens them inherits those names.
Deciding that in the generator means every composite has to suppress its
children's labels, at every level.
with_pin_labelsdoes it fordiff_pair,but the parameter stops there:
diff_pair_ibiasdoes not accept it and emitslabels of its own, so
opampcannot suppress anything. Each new composite isone more place that has to remember, and one that forgets fails silently --
which is how these three got here.
The reference netlist already states which names are pins. This honours that
when staging inputs: labels whose text is not a port of the top
.subcktaredropped from the staged copy.
parent flattened.
@cellkeys its cache on arguments: acontext manager or env var can hand back a component built under the
opposite setting.
and LEF/macro flows are unaffected.
capitalisation in practice (
vddvsVdd).Dropping every label is reported as a warning rather than passed over -- that
is a naming mismatch rather than inheritance, and silently leaving the layout
with no pins turns into a confusing failure further down. With no readable
ports, nothing is dropped.
Cost, measured over nine real cells (5-12 labels, 49-4913 polygons): 0.1-1.0 ms
to filter, ~27 ms including the GDS round trip, against 1-8 s per cell of LVS.
Once this lands,
with_pin_labelsand the pin-label halves of #103 and #110become unnecessary; #110 stops depending on #103 and can go in on its own.